Skip to content

feat(nextjs): Environment drift telemetry event for keyless applications #6522

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

heatlikeheatwave
Copy link
Contributor

@heatlikeheatwave heatlikeheatwave commented Aug 11, 2025

Description

  • Add new telemetry event KEYLESS_ENV_DRIFT_DETECTED to detect drift between publishable and secret keys in keyless apps and values in the .env file.
    Example event payload:
{
    "envPublishableKey":"pk_test_[redacted]",
    "envVarsMissing":false,
    "keylessFileHasKeys":true,
    "keylessPublishableKey":"pk_test_[redacted]",
    "publicKeyMatch":false,
    "secretKeyMatch":false
}
  • This event only fires once (controlled with the .clerk/.tmp/telemetry.json file) to prevent noise

Checklist

  • pnpm test runs as expected.
  • pnpm build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

Summary by CodeRabbit

  • New Features
    • Adds a server-only telemetry check that runs once to detect mismatches between environment variables and keyless configuration.
    • Automatically invoked during server-side provider initialization before rendering, with no changes to public APIs or UI.
    • Emits a single telemetry event when a mismatch is found; otherwise silent.
    • Non-blocking: errors are logged and do not affect app stability.

Copy link

changeset-bot bot commented Aug 11, 2025

🦋 Changeset detected

Latest commit: e5e7120

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@clerk/nextjs Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

vercel bot commented Aug 11, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Project Deployment Preview Comments Updated (UTC)
clerk-js-sandbox Ready Preview Comment Aug 12, 2025 9:37pm

@heatlikeheatwave heatlikeheatwave changed the title Heat/grow 664 investigate 10 drop between application created and dev feat(nextjs) Environment Drift Telemetry Event for Keyless Applications Aug 11, 2025
Copy link

pkg-pr-new bot commented Aug 11, 2025

Open in StackBlitz

@clerk/agent-toolkit

npm i https://pkg.pr.new/@clerk/agent-toolkit@6522

@clerk/astro

npm i https://pkg.pr.new/@clerk/astro@6522

@clerk/backend

npm i https://pkg.pr.new/@clerk/backend@6522

@clerk/chrome-extension

npm i https://pkg.pr.new/@clerk/chrome-extension@6522

@clerk/clerk-js

npm i https://pkg.pr.new/@clerk/clerk-js@6522

@clerk/dev-cli

npm i https://pkg.pr.new/@clerk/dev-cli@6522

@clerk/elements

npm i https://pkg.pr.new/@clerk/elements@6522

@clerk/clerk-expo

npm i https://pkg.pr.new/@clerk/clerk-expo@6522

@clerk/expo-passkeys

npm i https://pkg.pr.new/@clerk/expo-passkeys@6522

@clerk/express

npm i https://pkg.pr.new/@clerk/express@6522

@clerk/fastify

npm i https://pkg.pr.new/@clerk/fastify@6522

@clerk/localizations

npm i https://pkg.pr.new/@clerk/localizations@6522

@clerk/nextjs

npm i https://pkg.pr.new/@clerk/nextjs@6522

@clerk/nuxt

npm i https://pkg.pr.new/@clerk/nuxt@6522

@clerk/clerk-react

npm i https://pkg.pr.new/@clerk/clerk-react@6522

@clerk/react-router

npm i https://pkg.pr.new/@clerk/react-router@6522

@clerk/remix

npm i https://pkg.pr.new/@clerk/remix@6522

@clerk/shared

npm i https://pkg.pr.new/@clerk/shared@6522

@clerk/tanstack-react-start

npm i https://pkg.pr.new/@clerk/tanstack-react-start@6522

@clerk/testing

npm i https://pkg.pr.new/@clerk/testing@6522

@clerk/themes

npm i https://pkg.pr.new/@clerk/themes@6522

@clerk/types

npm i https://pkg.pr.new/@clerk/types@6522

@clerk/upgrade

npm i https://pkg.pr.new/@clerk/upgrade@6522

@clerk/vue

npm i https://pkg.pr.new/@clerk/vue@6522

commit: e5e7120

Copy link
Contributor

coderabbitai bot commented Aug 11, 2025

📝 Walkthrough

Walkthrough

Adds a server-only module that detects drift between NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY / CLERK_SECRET_KEY environment variables and a keyless Clerk configuration file. Exposes export async function detectKeylessEnvDrift(): Promise, dynamically imports keyless-node.js, reads the keyless file, computes matches/missing flags, and constructs a payload. Uses a file-based, single-emission guard (.clerk/.tmp/telemetry.json) and createClerkClientWithOptions to record a KEYLESS_ENV_DRIFT_DETECTED telemetry event (sampling rate 1) if drift is observed. All runtime errors are caught and logged. The app-router ClerkProvider now dynamically imports and awaits detectKeylessEnvDrift() (errors ignored) before rendering.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~15 minutes


🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (5)
packages/nextjs/src/server/keyless-telemetry.ts (5)

62-70: JSDoc mentions a NOT_DETECTED event that isn’t implemented

Either remove the NOT_DETECTED reference or implement the complementary event to match docs.

Suggested change:

- * If there's a mismatch, it fires a `KEYLESS_ENV_DRIFT_DETECTED` event.
- * For local testing purposes, it also fires a `KEYLESS_ENV_DRIFT_NOT_DETECTED` event when
- * keys exist and match the environment variables.
+ * If there's a mismatch, it fires a `KEYLESS_ENV_DRIFT_DETECTED` event.

2-3: Use node: scheme for built-ins (optional)

Prefer node:fs and node:path for clarity and tree-shaking friendliness in modern Node/TS setups.

Apply:

-import { promises as fs } from 'fs';
-import { join } from 'path';
+import { promises as fs } from 'node:fs';
+import { join } from 'node:path';

7-9: Narrow constant types

Mark event name and sampling rate as const to keep literal types where helpful.

Apply:

-const EVENT_KEYLESS_ENV_DRIFT_DETECTED = 'KEYLESS_ENV_DRIFT_DETECTED';
-const EVENT_SAMPLING_RATE = 1; // 100% sampling rate
+const EVENT_KEYLESS_ENV_DRIFT_DETECTED = 'KEYLESS_ENV_DRIFT_DETECTED' as const;
+const EVENT_SAMPLING_RATE = 1 as const; // 100% sampling rate

145-148: Use the package’s logger (if available) for consistent diagnostics

Console.warn works, but prefer the internal logger to integrate with existing log levels and silencing rules.


1-150: Tests missing for the new telemetry behavior

Please add tests covering:

  • drift detected -> emits event once (atomic flag ensures single emission),
  • no drift -> no event,
  • env missing + keyless file has keys -> early return,
  • missing keys in keyless file -> safe no-op,
  • masking behavior on keys.

Do you want me to scaffold unit tests (with fs and keyless-node mocks) for this module?

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 29a25f3 and d31a57b.

📒 Files selected for processing (2)
  • packages/nextjs/src/app-router/keyless-actions.ts (2 hunks)
  • packages/nextjs/src/server/keyless-telemetry.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels

Files:

  • packages/nextjs/src/app-router/keyless-actions.ts
  • packages/nextjs/src/server/keyless-telemetry.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

Use Prettier for consistent code formatting

Files:

  • packages/nextjs/src/app-router/keyless-actions.ts
  • packages/nextjs/src/server/keyless-telemetry.ts
packages/**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

TypeScript is required for all packages

Files:

  • packages/nextjs/src/app-router/keyless-actions.ts
  • packages/nextjs/src/server/keyless-telemetry.ts
packages/**/*.{ts,tsx,d.ts}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

Packages should export TypeScript types alongside runtime code

Files:

  • packages/nextjs/src/app-router/keyless-actions.ts
  • packages/nextjs/src/server/keyless-telemetry.ts
**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

Use proper TypeScript error types

**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details
Use protected for inheritance hierarchies
Use public explicitly for clarity in public APIs
Prefer readonly for properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Use const assertions for literal types: as const
Use satisfies operator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports: import type { ... } from ...
No any types without justification
Proper error handling with typed errors
Consistent use of readonly for immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)

Files:

  • packages/nextjs/src/app-router/keyless-actions.ts
  • packages/nextjs/src/server/keyless-telemetry.ts
**/*.{js,ts,tsx,jsx}

📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)

Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.

Files:

  • packages/nextjs/src/app-router/keyless-actions.ts
  • packages/nextjs/src/server/keyless-telemetry.ts
**/*

⚙️ CodeRabbit Configuration File

If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.

Files:

  • packages/nextjs/src/app-router/keyless-actions.ts
  • packages/nextjs/src/server/keyless-telemetry.ts
🧬 Code Graph Analysis (2)
packages/nextjs/src/app-router/keyless-actions.ts (1)
packages/nextjs/src/server/keyless-telemetry.ts (1)
  • detectKeylessEnvDrift (73-149)
packages/nextjs/src/server/keyless-telemetry.ts (3)
packages/nextjs/src/server/keyless-node.ts (1)
  • safeParseClerkFile (52-66)
packages/nextjs/src/server/createClerkClient.ts (1)
  • createClerkClientWithOptions (34-35)
packages/types/src/telemetry.ts (1)
  • TelemetryEventRaw (40-44)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Formatting | Dedupe | Changeset
  • GitHub Check: semgrep/ci
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
packages/nextjs/src/app-router/keyless-actions.ts (1)

65-67: Mismatch with summary: this runs unconditionally after result exists, not only on a “non-matching key” path

The AI summary states the call happens only on the “non-matching-key path,” but here it executes for every successful create/read. Confirm this is intended.

packages/nextjs/src/server/keyless-telemetry.ts (1)

1-150: No action required: Event name and env-var usage are compatible

  • TelemetryEvent.event is defined as a string, so KEYLESS_ENV_DRIFT_DETECTED is accepted by the telemetry system.
  • Across Next.js packages, environment variables are consistently read via process.env.* (e.g. in mergeNextClerkPropsWithEnv.ts and server/constants.ts); there is no shared utility to replace.

All checks pass—no changes needed.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (4)
packages/nextjs/src/app-router/keyless-actions.ts (2)

9-9: Avoid static import; prefer dynamic import to keep telemetry optional and out of the critical path

Import the telemetry module dynamically at call-site to avoid bundling it on every server action path and to adhere to the project's "Use dynamic imports for optional features" guideline.

Apply:

-import { detectKeylessEnvDrift } from '../server/keyless-telemetry';
+// Telemetry is imported dynamically at call-site

65-67: Do not await telemetry; use fire-and-forget dynamic import to avoid adding latency

Telemetry should never delay or fail the request. Trigger it non-blockingly and swallow errors.

Apply:

-  // Detect environment variable drift and fire telemetry events
-  await detectKeylessEnvDrift();
+  // Detect environment variable drift (non-blocking, dynamic import)
+  void import('../server/keyless-telemetry')
+    .then(m => m.detectKeylessEnvDrift())
+    .catch(() => {});
packages/nextjs/src/server/keyless-telemetry.ts (2)

96-98: Support all recognized Clerk env var names for publishable and secret keys

Per guidelines, support multiple variable names. Also consider CLERK_API_KEY as a fallback for the secret key.

Apply:

-    const envPublishableKey = process.env.NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY;
-    const envSecretKey = process.env.CLERK_SECRET_KEY;
+    const envPublishableKey =
+      process.env.NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY ??
+      process.env.CLERK_PUBLISHABLE_KEY;
+    const envSecretKey =
+      process.env.CLERK_SECRET_KEY ??
+      process.env.CLERK_API_KEY;

126-130: Guard against missing keys in keyless file before creating the client

safeParseClerkFile can return an object without both keys. Avoid passing undefined to the client.

Apply:

-    // Create a clerk client to access telemetry
-    const clerkClient = createClerkClientWithOptions({
-      publishableKey: keylessFile.publishableKey,
-      secretKey: keylessFile.secretKey,
-    });
+    // Guard against missing keys in keyless file
+    if (!keylessFile.publishableKey || !keylessFile.secretKey) {
+      return;
+    }
+    // Create a clerk client to access telemetry
+    const clerkClient = createClerkClientWithOptions({
+      publishableKey: keylessFile.publishableKey,
+      secretKey: keylessFile.secretKey,
+    });
🧹 Nitpick comments (2)
packages/nextjs/src/server/keyless-telemetry.ts (2)

68-70: Docstring mentions NOT_DETECTED event that isn’t implemented

Either implement the non-drift event or update the comment to reflect current behavior.

Apply:

- * For local testing purposes, it also fires a `KEYLESS_ENV_DRIFT_NOT_DETECTED` event when
- *
- * keys exist and match the environment variables.
+ * Note: currently only emits when drift is detected.

73-149: Please add unit tests to cover drift detection and single-emission semantics

Add tests for:

  • env vars missing + keyless file has keys => no event
  • env vars match file => no event (and no flag written)
  • only publishable mismatches => event once
  • only secret mismatches => event once
  • both mismatch => event once
  • concurrent calls => single event (atomic flag)
  • missing keys in file => early return, no client created

I can scaffold tests (e.g., using memfs to mock fs and a mock telemetry recorder). Let me know if you want me to open a follow-up PR with a test suite.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9a5305a and 3239d9c.

📒 Files selected for processing (2)
  • packages/nextjs/src/app-router/keyless-actions.ts (2 hunks)
  • packages/nextjs/src/server/keyless-telemetry.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels

Files:

  • packages/nextjs/src/app-router/keyless-actions.ts
  • packages/nextjs/src/server/keyless-telemetry.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

Use Prettier for consistent code formatting

Files:

  • packages/nextjs/src/app-router/keyless-actions.ts
  • packages/nextjs/src/server/keyless-telemetry.ts
packages/**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

TypeScript is required for all packages

Files:

  • packages/nextjs/src/app-router/keyless-actions.ts
  • packages/nextjs/src/server/keyless-telemetry.ts
packages/**/*.{ts,tsx,d.ts}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

Packages should export TypeScript types alongside runtime code

Files:

  • packages/nextjs/src/app-router/keyless-actions.ts
  • packages/nextjs/src/server/keyless-telemetry.ts
**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

Use proper TypeScript error types

**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details
Use protected for inheritance hierarchies
Use public explicitly for clarity in public APIs
Prefer readonly for properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Use const assertions for literal types: as const
Use satisfies operator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports: import type { ... } from ...
No any types without justification
Proper error handling with typed errors
Consistent use of readonly for immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)

Files:

  • packages/nextjs/src/app-router/keyless-actions.ts
  • packages/nextjs/src/server/keyless-telemetry.ts
**/*.{js,ts,tsx,jsx}

📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)

Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.

Files:

  • packages/nextjs/src/app-router/keyless-actions.ts
  • packages/nextjs/src/server/keyless-telemetry.ts
**/*

⚙️ CodeRabbit Configuration File

If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.

Files:

  • packages/nextjs/src/app-router/keyless-actions.ts
  • packages/nextjs/src/server/keyless-telemetry.ts
🧬 Code Graph Analysis (2)
packages/nextjs/src/app-router/keyless-actions.ts (1)
packages/nextjs/src/server/keyless-telemetry.ts (1)
  • detectKeylessEnvDrift (73-149)
packages/nextjs/src/server/keyless-telemetry.ts (3)
packages/nextjs/src/server/keyless-node.ts (1)
  • safeParseClerkFile (52-66)
packages/nextjs/src/server/createClerkClient.ts (1)
  • createClerkClientWithOptions (34-35)
packages/types/src/telemetry.ts (1)
  • TelemetryEventRaw (40-44)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Formatting | Dedupe | Changeset
  • GitHub Check: semgrep/ci
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: semgrep-cloud-platform/scan

@heatlikeheatwave heatlikeheatwave force-pushed the heat/grow-664-investigate-10-drop-between-application-created-and-dev branch from 3239d9c to 0cf7474 Compare August 12, 2025 18:59
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (4)
packages/nextjs/src/server/keyless-telemetry.ts (4)

11-18: Do not send raw keys in telemetry; mask values and fix unsafe cast

The payload currently includes raw publishable keys and casts a possibly-undefined env var to string. This risks PII leakage and hides type issues. Mask the keys and make these fields optional, aligning with the sample payload that shows redacted values.

Apply:

-type EventKeylessEnvDriftPayload = {
+interface EventKeylessEnvDriftPayload {
   publicKeyMatch: boolean;
   secretKeyMatch: boolean;
   envVarsMissing: boolean;
   keylessFileHasKeys: boolean;
-  keylessPublishableKey: string;
-  envPublishableKey: string;
-};
+  keylessPublishableKeyMasked?: string;
+  envPublishableKeyMasked?: string;
+}
-    const payload: EventKeylessEnvDriftPayload = {
+    const payload: EventKeylessEnvDriftPayload = {
       publicKeyMatch,
       secretKeyMatch,
       envVarsMissing,
       keylessFileHasKeys,
-      keylessPublishableKey: keylessFile.publishableKey,
-      envPublishableKey: envPublishableKey as string,
+      keylessPublishableKeyMasked: maskKeyForTelemetry(keylessFile.publishableKey),
+      envPublishableKeyMasked: maskKeyForTelemetry(envPublishableKey),
     };

Add helper outside the shown ranges:

function maskKeyForTelemetry(key?: string): string | undefined {
  if (!key) return undefined;
  const head = key.slice(0, 8);
  const tail = key.slice(-4);
  return `${head}${tail}`;
}

Also applies to: 117-124


27-37: Eliminate race in “check then mark”; use atomic create-if-not-exists for single emission

The two-phase check with hasTelemetryEventBeenFired + markTelemetryEventAsFired is racy under concurrency and can emit duplicates. Replace with an atomic writer using { flag: 'wx' } and emit only if this call wins.

Replace the helpers:

-/**
- * Checks if the telemetry event has already been fired by looking for a flag file.
- */
-async function hasTelemetryEventBeenFired(): Promise<boolean> {
-  try {
-    await fs.access(getTelemetryFlagFilePath());
-    return true;
-  } catch {
-    return false;
-  }
-}
-
-/**
- * Creates a flag file to mark that the telemetry event has been fired.
- */
-async function markTelemetryEventAsFired(): Promise<void> {
-  try {
-    // Ensure the directory exists
-    const dir = join(process.cwd(), '.clerk/.tmp');
-    await fs.mkdir(dir, { recursive: true });
-
-    // Create the flag file with timestamp
-    const flagData = {
-      firedAt: new Date().toISOString(),
-      event: EVENT_KEYLESS_ENV_DRIFT_DETECTED,
-    };
-
-    await fs.writeFile(getTelemetryFlagFilePath(), JSON.stringify(flagData, null, 2));
-  } catch (error) {
-    // Silently handle errors to avoid breaking the application
-    console.warn('Failed to create telemetry flag file:', error);
-  }
-}
+/**
+ * Atomically attempts to mark the telemetry event as fired (single-emitter guard).
+ * Returns true only if this call created the flag.
+ */
+async function tryMarkTelemetryEventAsFired(): Promise<boolean> {
+  try {
+    const dir = join(process.cwd(), '.clerk/.tmp');
+    await fs.mkdir(dir, { recursive: true });
+    const flagData = {
+      firedAt: new Date().toISOString(),
+      event: EVENT_KEYLESS_ENV_DRIFT_DETECTED,
+    };
+    await fs.writeFile(getTelemetryFlagFilePath(), JSON.stringify(flagData, null, 2), { flag: 'wx' });
+    return true;
+  } catch (error: unknown) {
+    if ((error as { code?: string })?.code === 'EEXIST') {
+      return false;
+    }
+    console.warn('Failed to create telemetry flag file:', error);
+    return false;
+  }
+}

Remove the racy pre-check:

-    // Check if telemetry event has already been fired
-    if (await hasTelemetryEventBeenFired()) {
-      return;
-    }

Acquire the single-emission lock only when emitting, and remove the final mark:

     if (hasDrift) {
-      // Fire drift detected event
+      // Acquire single-emission lock. Only the winner should emit.
+      const shouldEmit = await tryMarkTelemetryEventAsFired();
+      if (!shouldEmit) {
+        return;
+      }
+      // Fire drift detected event
       const driftDetectedEvent: TelemetryEventRaw<EventKeylessEnvDriftPayload> = {
         event: EVENT_KEYLESS_ENV_DRIFT_DETECTED,
         eventSamplingRate: EVENT_SAMPLING_RATE,
         payload,
       };
 
       clerkClient.telemetry?.record(driftDetectedEvent);
-
-      // Mark the telemetry event as fired to prevent future executions
-      await markTelemetryEventAsFired();
     }

Also applies to: 39-59, 79-84, 132-144


96-97: Support CLERK_PUBLISHABLE_KEY as a fallback for publishable key env

Per guidelines, support multiple env var names. Read CLERK_PUBLISHABLE_KEY if NEXT_PUBLIC_… is not set.

-    const envPublishableKey = process.env.NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY;
+    const envPublishableKey =
+      process.env.NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY ??
+      process.env.CLERK_PUBLISHABLE_KEY;

126-131: Guard against missing keys in keyless file before creating the client

safeParseClerkFile can yield an object missing keys; avoid passing undefined to the client.

-    // Create a clerk client to access telemetry
+    // Create a clerk client to access telemetry
+    if (!keylessFile.publishableKey || !keylessFile.secretKey) {
+      return;
+    }
     const clerkClient = createClerkClientWithOptions({
       publishableKey: keylessFile.publishableKey,
       secretKey: keylessFile.secretKey,
     });
🧹 Nitpick comments (3)
packages/nextjs/src/server/keyless-telemetry.ts (3)

67-72: Docstring claims a NOT_DETECTED event that isn't implemented

Either implement the NOT_DETECTED event or update the comment to reflect the current behavior (emit only on drift).

- * If there's a mismatch, it fires a `KEYLESS_ENV_DRIFT_DETECTED` event.
- * For local testing purposes, it also fires a `KEYLESS_ENV_DRIFT_NOT_DETECTED` event when
- * keys exist and match the environment variables.
- *
- * The telemetry event will only fire once per application instance to avoid noise.
+ * If there's a mismatch, it fires a `KEYLESS_ENV_DRIFT_DETECTED` event.
+ * The telemetry event will only fire once per application instance to avoid noise.

Would you like me to add the NOT_DETECTED event instead?


55-58: Type catch variables as unknown (TS best practice)

-  } catch (error) {
+  } catch (error: unknown) {
     // Silently handle errors to avoid breaking the application
     console.warn('Failed to create telemetry flag file:', error);
   }
-  } catch (error) {
+  } catch (error: unknown) {
     // Silently handle errors to avoid breaking the application
     console.warn('Failed to detect keyless environment drift:', error);
   }

Also applies to: 145-148


73-149: Please add unit/integration tests for this feature

No tests are included. Add coverage for:

  • Emits KEYLESS_ENV_DRIFT_DETECTED exactly once under concurrent calls (atomic file guard)
  • Masks keys in payload (no raw keys in telemetry)
  • Fallback to CLERK_PUBLISHABLE_KEY when NEXT_PUBLIC_* is absent
  • No emission when keyless file missing or missing keys
  • Match logic requires both sides present

I can scaffold tests using a temporary directory for .clerk/.tmp and mock createClerkClientWithOptions.telemetry.record. Do you want me to open a follow-up PR with tests?

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3239d9c and 0cf7474.

📒 Files selected for processing (2)
  • packages/nextjs/src/app-router/keyless-actions.ts (2 hunks)
  • packages/nextjs/src/server/keyless-telemetry.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/nextjs/src/app-router/keyless-actions.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels

Files:

  • packages/nextjs/src/server/keyless-telemetry.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

Use Prettier for consistent code formatting

Files:

  • packages/nextjs/src/server/keyless-telemetry.ts
packages/**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

TypeScript is required for all packages

Files:

  • packages/nextjs/src/server/keyless-telemetry.ts
packages/**/*.{ts,tsx,d.ts}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

Packages should export TypeScript types alongside runtime code

Files:

  • packages/nextjs/src/server/keyless-telemetry.ts
**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

Use proper TypeScript error types

**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details
Use protected for inheritance hierarchies
Use public explicitly for clarity in public APIs
Prefer readonly for properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Use const assertions for literal types: as const
Use satisfies operator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports: import type { ... } from ...
No any types without justification
Proper error handling with typed errors
Consistent use of readonly for immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)

Files:

  • packages/nextjs/src/server/keyless-telemetry.ts
**/*.{js,ts,tsx,jsx}

📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)

Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.

Files:

  • packages/nextjs/src/server/keyless-telemetry.ts
**/*

⚙️ CodeRabbit Configuration File

If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.

Files:

  • packages/nextjs/src/server/keyless-telemetry.ts
🧬 Code Graph Analysis (1)
packages/nextjs/src/server/keyless-telemetry.ts (3)
packages/nextjs/src/server/keyless-node.ts (1)
  • safeParseClerkFile (52-66)
packages/nextjs/src/server/createClerkClient.ts (1)
  • createClerkClientWithOptions (34-35)
packages/types/src/telemetry.ts (1)
  • TelemetryEventRaw (40-44)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Formatting | Dedupe | Changeset
  • GitHub Check: semgrep/ci
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (1)
packages/nextjs/src/server/keyless-telemetry.ts (1)

100-101: Clarify envVarsMissing semantics (any vs both missing) and adjust logic accordingly

Currently it means “both missing” (&&). If the intent is to signal misconfiguration when either is missing, use ||. Also consider naming for clarity (e.g., anyEnvVarMissing, bothEnvVarsMissing).

Option A (any missing):

-    const envVarsMissing = !envPublishableKey && !envSecretKey;
+    const envVarsMissing = !envPublishableKey || !envSecretKey;

Option B (keep both missing) but prevent false “match” results; see next comment on match computation.

@heatlikeheatwave heatlikeheatwave force-pushed the heat/grow-664-investigate-10-drop-between-application-created-and-dev branch from 69f6547 to 83f4835 Compare August 12, 2025 20:36
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

♻️ Duplicate comments (4)
packages/nextjs/src/server/keyless-telemetry.ts (4)

95-96: Support CLERK_PUBLISHABLE_KEY as a fallback for the publishable key

Per the project guidelines, recognize multiple env var names. Also check CLERK_PUBLISHABLE_KEY.

-    const envPublishableKey = process.env.NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY;
+    const envPublishableKey =
+      process.env.NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY ?? process.env.CLERK_PUBLISHABLE_KEY;

107-112: Don’t treat “both missing” as a match; require presence on both sides before equality

undefined === undefined currently yields a “match,” masking missing data.

-    const publicKeyMatch = Boolean(envPublishableKey === keylessFile?.publishableKey);
+    const publicKeyMatch = Boolean(
+      envPublishableKey && keylessFile?.publishableKey && envPublishableKey === keylessFile.publishableKey,
+    );
 
-    const secretKeyMatch = Boolean(envSecretKey === keylessFile?.secretKey);
+    const secretKeyMatch = Boolean(
+      envSecretKey && keylessFile?.secretKey && envSecretKey === keylessFile.secretKey,
+    );

11-18: Do not send raw keys in telemetry; mask values and fix unsafe cast

Publishing full keys (even publishable) is unnecessary and risky; also, casting possibly-undefined envPublishableKey to string hides issues. Mask keys and make these fields optional.

-type EventKeylessEnvDriftPayload = {
+interface EventKeylessEnvDriftPayload {
   publicKeyMatch: boolean;
   secretKeyMatch: boolean;
   envVarsMissing: boolean;
   keylessFileHasKeys: boolean;
-  keylessPublishableKey: string;
-  envPublishableKey: string;
-};
+  keylessPublishableKeyMasked?: string;
+  envPublishableKeyMasked?: string;
+}
@@
-    const payload: EventKeylessEnvDriftPayload = {
+    const payload: EventKeylessEnvDriftPayload = {
       publicKeyMatch,
       secretKeyMatch,
       envVarsMissing,
       keylessFileHasKeys,
-      keylessPublishableKey: keylessFile.publishableKey,
-      envPublishableKey: envPublishableKey as string,
+      keylessPublishableKeyMasked: maskKeyForTelemetry(keylessFile.publishableKey),
+      envPublishableKeyMasked: maskKeyForTelemetry(envPublishableKey),
     };

Add this helper outside the shown ranges:

function maskKeyForTelemetry(key?: string): string | undefined {
  if (!key) return undefined;
  const head = key.slice(0, 8);
  const tail = key.slice(-4);
  return `${head}${tail}`;
}

Also applies to: 116-123


125-129: Guard client creation when keyless file lacks keys

Avoid passing undefined to createClerkClientWithOptions.

-    // Create a clerk client to access telemetry
-    const clerkClient = createClerkClientWithOptions({
-      publishableKey: keylessFile.publishableKey,
-      secretKey: keylessFile.secretKey,
-    });
+    // Create a clerk client to access telemetry
+    if (!keylessFile.publishableKey || !keylessFile.secretKey) {
+      return;
+    }
+    const clerkClient = createClerkClientWithOptions({
+      publishableKey: keylessFile.publishableKey,
+      secretKey: keylessFile.secretKey,
+    });
🧹 Nitpick comments (2)
packages/nextjs/src/server/keyless-telemetry.ts (2)

28-30: Minor: use join with path segments for cross‑platform correctness

Joining a single string containing separators is usually fine, but using path segments is more robust.

-const TELEMETRY_FLAG_FILE = '.clerk/.tmp/telemetry.json';
+const TELEMETRY_FLAG_FILE = 'telemetry.json';
@@
-function getTelemetryFlagFilePath(): string {
-  return join(process.cwd(), TELEMETRY_FLAG_FILE);
-}
+function getTelemetryFlagFilePath(): string {
+  return join(process.cwd(), '.clerk', '.tmp', TELEMETRY_FLAG_FILE);
+}

Also applies to: 9-9


77-149: Add tests for drift detection and single‑emission guard

New behavior lacks tests. Add unit tests that:

  • Simulate keyless file present and env vars absent (no drift, no emission).
  • Simulate drift and verify a single emission even under concurrent calls (mock fs to pre-create the flag and assert no duplicate record).
  • Verify masking logic (payload never contains unmasked keys).
  • Verify errors are swallowed (import/read/write errors don’t throw).

I can help scaffold these with fs mocks and a fake telemetry client if you want.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0cf7474 and 83f4835.

📒 Files selected for processing (3)
  • packages/nextjs/src/app-router/server/ClerkProvider.tsx (1 hunks)
  • packages/nextjs/src/app-router/server/keyless-provider.tsx (1 hunks)
  • packages/nextjs/src/server/keyless-telemetry.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels

Files:

  • packages/nextjs/src/app-router/server/keyless-provider.tsx
  • packages/nextjs/src/app-router/server/ClerkProvider.tsx
  • packages/nextjs/src/server/keyless-telemetry.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

Use Prettier for consistent code formatting

Files:

  • packages/nextjs/src/app-router/server/keyless-provider.tsx
  • packages/nextjs/src/app-router/server/ClerkProvider.tsx
  • packages/nextjs/src/server/keyless-telemetry.ts
packages/**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

TypeScript is required for all packages

Files:

  • packages/nextjs/src/app-router/server/keyless-provider.tsx
  • packages/nextjs/src/app-router/server/ClerkProvider.tsx
  • packages/nextjs/src/server/keyless-telemetry.ts
packages/**/*.{ts,tsx,d.ts}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

Packages should export TypeScript types alongside runtime code

Files:

  • packages/nextjs/src/app-router/server/keyless-provider.tsx
  • packages/nextjs/src/app-router/server/ClerkProvider.tsx
  • packages/nextjs/src/server/keyless-telemetry.ts
**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

Use proper TypeScript error types

**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details
Use protected for inheritance hierarchies
Use public explicitly for clarity in public APIs
Prefer readonly for properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Use const assertions for literal types: as const
Use satisfies operator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports: import type { ... } from ...
No any types without justification
Proper error handling with typed errors
Consistent use of readonly for immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)

Files:

  • packages/nextjs/src/app-router/server/keyless-provider.tsx
  • packages/nextjs/src/app-router/server/ClerkProvider.tsx
  • packages/nextjs/src/server/keyless-telemetry.ts
**/*.{jsx,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

**/*.{jsx,tsx}: Use error boundaries in React components
Minimize re-renders in React components

**/*.{jsx,tsx}: Always use functional components with hooks instead of class components
Follow PascalCase naming for components: UserProfile, NavigationMenu
Keep components focused on a single responsibility - split large components
Limit component size to 150-200 lines; extract logic into custom hooks
Use composition over inheritance - prefer smaller, composable components
Export components as named exports for better tree-shaking
One component per file with matching filename and component name
Use useState for simple state management
Use useReducer for complex state logic
Implement proper state initialization
Use proper state updates with callbacks
Implement proper state cleanup
Use Context API for theme/authentication
Implement proper state selectors
Use proper state normalization
Implement proper state persistence
Use React.memo for expensive components
Implement proper useCallback for handlers
Use proper useMemo for expensive computations
Implement proper virtualization for lists
Use proper code splitting with React.lazy
Implement proper cleanup in useEffect
Use proper refs for DOM access
Implement proper event listener cleanup
Use proper abort controllers for fetch
Implement proper subscription cleanup
Use proper HTML elements
Implement proper ARIA attributes
Use proper heading hierarchy
Implement proper form labels
Use proper button types
Implement proper focus management
Use proper keyboard shortcuts
Implement proper tab order
Use proper skip links
Implement proper focus traps
Implement proper error boundaries
Use proper error logging
Implement proper error recovery
Use proper error messages
Implement proper error fallbacks
Use proper form validation
Implement proper error states
Use proper error messages
Implement proper form submission
Use proper form reset
Use proper component naming
Implement proper file naming
Use proper prop naming
Implement proper...

Files:

  • packages/nextjs/src/app-router/server/keyless-provider.tsx
  • packages/nextjs/src/app-router/server/ClerkProvider.tsx
**/*.{js,ts,tsx,jsx}

📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)

Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.

Files:

  • packages/nextjs/src/app-router/server/keyless-provider.tsx
  • packages/nextjs/src/app-router/server/ClerkProvider.tsx
  • packages/nextjs/src/server/keyless-telemetry.ts
**/*.tsx

📄 CodeRabbit Inference Engine (.cursor/rules/react.mdc)

**/*.tsx: Use proper type definitions for props and state
Leverage TypeScript's type inference where possible
Use proper event types for handlers
Implement proper generic types for reusable components
Use proper type guards for conditional rendering

Files:

  • packages/nextjs/src/app-router/server/keyless-provider.tsx
  • packages/nextjs/src/app-router/server/ClerkProvider.tsx
**/*

⚙️ CodeRabbit Configuration File

If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.

Files:

  • packages/nextjs/src/app-router/server/keyless-provider.tsx
  • packages/nextjs/src/app-router/server/ClerkProvider.tsx
  • packages/nextjs/src/server/keyless-telemetry.ts
🧬 Code Graph Analysis (3)
packages/nextjs/src/app-router/server/keyless-provider.tsx (1)
packages/nextjs/src/server/keyless-telemetry.ts (1)
  • detectKeylessEnvDrift (77-149)
packages/nextjs/src/app-router/server/ClerkProvider.tsx (1)
packages/nextjs/src/server/keyless-telemetry.ts (1)
  • detectKeylessEnvDrift (77-149)
packages/nextjs/src/server/keyless-telemetry.ts (3)
packages/nextjs/src/server/keyless-node.ts (1)
  • safeParseClerkFile (52-66)
packages/nextjs/src/server/createClerkClient.ts (1)
  • createClerkClientWithOptions (34-35)
packages/types/src/telemetry.ts (1)
  • TelemetryEventRaw (40-44)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
  • GitHub Check: Integration Tests (elements, chrome)
  • GitHub Check: Integration Tests (quickstart, chrome)
  • GitHub Check: Integration Tests (nextjs, chrome, 14)
  • GitHub Check: Integration Tests (machine, chrome)
  • GitHub Check: Integration Tests (billing, chrome)
  • GitHub Check: Integration Tests (nextjs, chrome, 15)
  • GitHub Check: Integration Tests (sessions, chrome)
  • GitHub Check: Integration Tests (localhost, chrome)
  • GitHub Check: Integration Tests (ap-flows, chrome)
  • GitHub Check: Static analysis
  • GitHub Check: semgrep-cloud-platform/scan

@heatlikeheatwave heatlikeheatwave force-pushed the heat/grow-664-investigate-10-drop-between-application-created-and-dev branch from 83f4835 to f74ec35 Compare August 12, 2025 21:01
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (6)
packages/nextjs/src/server/keyless-telemetry.ts (6)

3-3: Prevent ENOENT when creating the flag file: import dirname from 'path'

You need dirname to ensure the parent directory exists before atomic write.

Apply:

-import { join } from 'path';
+import { join, dirname } from 'path';

43-50: Ensure atomic flag write creates parent directory; otherwise ENOENT prevents emission

Atomic write with 'wx' is good, but without mkdir -p on the parent directory, the write can fail with ENOENT and you’ll never emit.

Apply:

 async function tryMarkTelemetryEventAsFired(): Promise<boolean> {
   try {
     const flagData = {
       firedAt: new Date().toISOString(),
       event: EVENT_KEYLESS_ENV_DRIFT_DETECTED,
     };
-    await fs.writeFile(getTelemetryFlagFilePath(), JSON.stringify(flagData, null, 2), { flag: 'wx' });
+    const filePath = getTelemetryFlagFilePath();
+    await fs.mkdir(dirname(filePath), { recursive: true });
+    await fs.writeFile(filePath, JSON.stringify(flagData, null, 2), { flag: 'wx' });
     return true;

95-96: Support CLERK_PUBLISHABLE_KEY fallback per guidelines

Per project guidelines, support multiple env var names for configuration. Add CLERK_PUBLISHABLE_KEY as a fallback.

Apply:

-    const envPublishableKey = process.env.NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY;
+    const envPublishableKey =
+      process.env.NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY ?? process.env.CLERK_PUBLISHABLE_KEY;

11-18: Do not send raw keys; mask values and avoid unsafe empty-string cast in payload type

Sending full publishable keys in telemetry is unnecessary and risky. Also, forcing possibly-undefined values to '' hides real "missing" states.

Apply:

-type EventKeylessEnvDriftPayload = {
+interface EventKeylessEnvDriftPayload {
   publicKeyMatch: boolean;
   secretKeyMatch: boolean;
   envVarsMissing: boolean;
   keylessFileHasKeys: boolean;
-  keylessPublishableKey: string;
-  envPublishableKey: string;
-};
+  keylessPublishableKeyMasked?: string;
+  envPublishableKeyMasked?: string;
+}

Add helper (outside shown ranges):

function maskKeyForTelemetry(key?: string): string | undefined {
  if (!key) return undefined;
  const head = key.slice(0, 8);
  const tail = key.slice(-4);
  return `${head}${tail}`;
}

117-124: Mask publishable keys in telemetry payload; don’t coerce undefined to ''

Replace raw key fields with masked, optional values.

Apply:

-    const payload: EventKeylessEnvDriftPayload = {
+    const payload: EventKeylessEnvDriftPayload = {
       publicKeyMatch,
       secretKeyMatch,
       envVarsMissing,
       keylessFileHasKeys,
-      keylessPublishableKey: keylessFile.publishableKey,
-      envPublishableKey: envPublishableKey ?? '',
+      keylessPublishableKeyMasked: maskKeyForTelemetry(keylessFile.publishableKey),
+      envPublishableKeyMasked: maskKeyForTelemetry(envPublishableKey),
     };

126-145: Avoid initializing Clerk client with undefined keys; move client creation inside emission block

Currently, the client is created even when no drift exists, and it may be created with undefined keys in mixed/missing scenarios.

Apply:

-    // Create a clerk client to access telemetry
-    const clerkClient = createClerkClientWithOptions({
-      publishableKey: keylessFile.publishableKey,
-      secretKey: keylessFile.secretKey,
-    });
-
-    if (hasDrift) {
-      const shouldFireEvent = await tryMarkTelemetryEventAsFired();
-
-      if (shouldFireEvent) {
-        // Fire drift detected event only if we successfully created the flag
-        const driftDetectedEvent: TelemetryEventRaw<EventKeylessEnvDriftPayload> = {
-          event: EVENT_KEYLESS_ENV_DRIFT_DETECTED,
-          eventSamplingRate: EVENT_SAMPLING_RATE,
-          payload,
-        };
-
-        clerkClient.telemetry?.record(driftDetectedEvent);
-      }
-    }
+    if (hasDrift) {
+      const shouldFireEvent = await tryMarkTelemetryEventAsFired();
+      if (!shouldFireEvent) {
+        return;
+      }
+      // Avoid initializing the client with missing keys
+      if (!keylessFile.publishableKey || !keylessFile.secretKey) {
+        return;
+      }
+      // Create a clerk client to access telemetry
+      const clerkClient = createClerkClientWithOptions({
+        publishableKey: keylessFile.publishableKey,
+        secretKey: keylessFile.secretKey,
+      });
+      // Fire drift detected event
+      const driftDetectedEvent: TelemetryEventRaw<EventKeylessEnvDriftPayload> = {
+        event: EVENT_KEYLESS_ENV_DRIFT_DETECTED,
+        eventSamplingRate: EVENT_SAMPLING_RATE,
+        payload,
+      };
+      clerkClient.telemetry?.record(driftDetectedEvent);
+    }
🧹 Nitpick comments (2)
packages/nextjs/src/server/keyless-telemetry.ts (2)

146-149: Type the error in catch for consistency

Be explicit with unknown.

Apply:

-  } catch (error) {
+  } catch (error: unknown) {

1-151: Add unit tests for drift detection and single-emission guard

No tests were added. Please cover core scenarios.

Suggested cases:

  • Server guard: early return when window is defined.
  • No config file: detectKeylessEnvDrift returns without side effects.
  • Keyless mode: env vars missing AND keyless file has both keys => no telemetry.
  • Drift detected (publishable mismatch and/or secret mismatch): telemetry event recorded with masked keys; ensure payload booleans are correct.
  • Single emission: concurrent calls race -> only one attempts to create flag and record; others exit due to EEXIST. Mock fs.promises to simulate EEXIST.
  • Missing keys in keyless file: function does not initialize Clerk client and exits quietly.
  • Env var fallback: when NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY is absent but CLERK_PUBLISHABLE_KEY present, detection still works.

I can scaffold Vitest tests with fs mocks and a mock for ./keyless-node.js’s safeParseClerkFile, and stub createClerkClientWithOptions to capture telemetry.record calls. Want me to push a test file under packages/nextjs/src/server/tests/keyless-telemetry.test.ts?

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 83f4835 and f74ec35.

📒 Files selected for processing (2)
  • packages/nextjs/src/app-router/server/ClerkProvider.tsx (1 hunks)
  • packages/nextjs/src/server/keyless-telemetry.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/nextjs/src/app-router/server/ClerkProvider.tsx
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels

Files:

  • packages/nextjs/src/server/keyless-telemetry.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

Use Prettier for consistent code formatting

Files:

  • packages/nextjs/src/server/keyless-telemetry.ts
packages/**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

TypeScript is required for all packages

Files:

  • packages/nextjs/src/server/keyless-telemetry.ts
packages/**/*.{ts,tsx,d.ts}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

Packages should export TypeScript types alongside runtime code

Files:

  • packages/nextjs/src/server/keyless-telemetry.ts
**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

Use proper TypeScript error types

**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details
Use protected for inheritance hierarchies
Use public explicitly for clarity in public APIs
Prefer readonly for properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Use const assertions for literal types: as const
Use satisfies operator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports: import type { ... } from ...
No any types without justification
Proper error handling with typed errors
Consistent use of readonly for immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)

Files:

  • packages/nextjs/src/server/keyless-telemetry.ts
**/*.{js,ts,tsx,jsx}

📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)

Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.

Files:

  • packages/nextjs/src/server/keyless-telemetry.ts
**/*

⚙️ CodeRabbit Configuration File

If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.

Files:

  • packages/nextjs/src/server/keyless-telemetry.ts
🧬 Code Graph Analysis (1)
packages/nextjs/src/server/keyless-telemetry.ts (3)
packages/nextjs/src/server/keyless-node.ts (1)
  • safeParseClerkFile (52-66)
packages/nextjs/src/server/createClerkClient.ts (1)
  • createClerkClientWithOptions (34-35)
packages/types/src/telemetry.ts (1)
  • TelemetryEventRaw (40-44)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Formatting | Dedupe | Changeset
  • GitHub Check: semgrep/ci
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (2)
packages/nextjs/src/server/keyless-telemetry.ts (2)

9-9: Flag file path differs from PR objective; align naming

PR summary specifies .clerk/.tmp/keyless-telemetry-fired.json; code uses telemetry.json. Align to avoid confusion and duplicate emissions across versions.

Apply:

-const TELEMETRY_FLAG_FILE = '.clerk/.tmp/telemetry.json';
+const TELEMETRY_FLAG_FILE = '.clerk/.tmp/keyless-telemetry-fired.json';

Please confirm there are no other consumers relying on the old filename before changing.


107-113: Good fix: only treat keys as matching when both sides are present

This prevents undefined === undefined from yielding false positives.

@heatlikeheatwave heatlikeheatwave force-pushed the heat/grow-664-investigate-10-drop-between-application-created-and-dev branch from f74ec35 to 0c5b8fe Compare August 12, 2025 21:26
@heatlikeheatwave heatlikeheatwave enabled auto-merge (squash) August 12, 2025 21:29
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

♻️ Duplicate comments (1)
packages/nextjs/src/server/keyless-telemetry.ts (1)

105-108: Support both NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY and CLERK_PUBLISHABLE_KEY
The guidelines require supporting multiple Clerk env var names. Add the common fallback for publishable key.

Apply:

-    const envPublishableKey = process.env.NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY;
+    const envPublishableKey =
+      process.env.NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY ?? process.env.CLERK_PUBLISHABLE_KEY;
     const envSecretKey = process.env.CLERK_SECRET_KEY;
🧹 Nitpick comments (2)
packages/nextjs/src/server/keyless-telemetry.ts (2)

162-179: Avoid unnecessary client initialization when the flag indicates “don’t emit”
Create the client only if we win the single-emission check. Saves work in the common path when the flag exists.

Apply:

-    // Create a clerk client to access telemetry
-    const clerkClient = createClerkClientWithOptions({
-      publishableKey: keylessFile.publishableKey,
-      secretKey: keylessFile.secretKey,
-    });
-
-    const shouldFireEvent = await tryMarkTelemetryEventAsFired();
-
-    if (shouldFireEvent) {
-      // Fire drift detected event only if we successfully created the flag
+    // Acquire single-emission flag before initializing the client to avoid unnecessary work
+    const shouldFireEvent = await tryMarkTelemetryEventAsFired();
+
+    if (shouldFireEvent) {
+      // Create a clerk client to access telemetry
+      const clerkClient = createClerkClientWithOptions({
+        publishableKey: keylessFile.publishableKey,
+        secretKey: keylessFile.secretKey,
+      });
+      // Fire drift detected event only if we successfully created the flag
       const driftDetectedEvent: TelemetryEventRaw<EventKeylessEnvDriftPayload> = {
         event: EVENT_KEYLESS_ENV_DRIFT_DETECTED,
         eventSamplingRate: EVENT_SAMPLING_RATE,
         payload,
       };
 
       clerkClient.telemetry?.record(driftDetectedEvent);
     }

88-184: Please add tests for drift detection scenarios and single-emission guard
No tests were added. Given the logic complexity and concurrency guard, add coverage for:

  • env missing + keyless present → no emit
  • env present + keyless missing → no emit
  • both present and equal → no emit
  • both present and mismatch (pub only, sec only, and both) → emit once
  • flag file already exists → no emit
  • flag directory missing → mkdir path then emit
  • read-only filesystem (write failing with non-EEXIST) → no emit, warning logged

I can scaffold tests for these cases. Do you want me to open a follow-up PR with a test suite?

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f74ec35 and 0c5b8fe.

📒 Files selected for processing (2)
  • packages/nextjs/src/app-router/server/ClerkProvider.tsx (1 hunks)
  • packages/nextjs/src/server/keyless-telemetry.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/nextjs/src/app-router/server/ClerkProvider.tsx
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels

Files:

  • packages/nextjs/src/server/keyless-telemetry.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

Use Prettier for consistent code formatting

Files:

  • packages/nextjs/src/server/keyless-telemetry.ts
packages/**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

TypeScript is required for all packages

Files:

  • packages/nextjs/src/server/keyless-telemetry.ts
packages/**/*.{ts,tsx,d.ts}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

Packages should export TypeScript types alongside runtime code

Files:

  • packages/nextjs/src/server/keyless-telemetry.ts
**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

Use proper TypeScript error types

**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details
Use protected for inheritance hierarchies
Use public explicitly for clarity in public APIs
Prefer readonly for properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Use const assertions for literal types: as const
Use satisfies operator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports: import type { ... } from ...
No any types without justification
Proper error handling with typed errors
Consistent use of readonly for immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)

Files:

  • packages/nextjs/src/server/keyless-telemetry.ts
**/*.{js,ts,tsx,jsx}

📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)

Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.

Files:

  • packages/nextjs/src/server/keyless-telemetry.ts
**/*

⚙️ CodeRabbit Configuration File

If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.

Files:

  • packages/nextjs/src/server/keyless-telemetry.ts
🧬 Code Graph Analysis (1)
packages/nextjs/src/server/keyless-telemetry.ts (3)
packages/nextjs/src/server/keyless-node.ts (1)
  • safeParseClerkFile (52-66)
packages/nextjs/src/server/createClerkClient.ts (1)
  • createClerkClientWithOptions (34-35)
packages/types/src/telemetry.ts (1)
  • TelemetryEventRaw (40-44)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: CodeQL
  • GitHub Check: Formatting | Dedupe | Changeset
  • GitHub Check: semgrep/ci
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (2)
packages/nextjs/src/server/keyless-telemetry.ts (2)

43-64: Atomic single-emitter guard looks solid
Good use of ‘wx’ with a recursive mkdir and EEXIST handling. This eliminates the race in “check-then-mark”.


135-147: Correct, non-naive match checks
Requiring both sides to be present before declaring a match avoids “undefined === undefined” false positives. This is the right approach.

@heatlikeheatwave heatlikeheatwave changed the title feat(nextjs) Environment Drift Telemetry Event for Keyless Applications feat(nextjs): Environment drift telemetry event for keyless applications Aug 12, 2025
@heatlikeheatwave heatlikeheatwave force-pushed the heat/grow-664-investigate-10-drop-between-application-created-and-dev branch from 0c5b8fe to e5e7120 Compare August 12, 2025 21:35
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (2)
packages/nextjs/src/server/keyless-telemetry.ts (2)

105-108: Support both NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY and CLERK_PUBLISHABLE_KEY

Per project guidelines, support multiple recognized env var names. Without the fallback, server-side configs using CLERK_PUBLISHABLE_KEY will be misdetected.

-    const envPublishableKey = process.env.NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY;
+    const envPublishableKey =
+      process.env.NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY ?? process.env.CLERK_PUBLISHABLE_KEY;
     const envSecretKey = process.env.CLERK_SECRET_KEY;

153-160: Align payload with masked/optional key fields; avoid empty-string placeholders

Payload currently transmits raw publishable keys and uses '' as placeholder for missing values. Replace with masked, optional fields to avoid leaking key material and to faithfully represent absence.

-    const payload: EventKeylessEnvDriftPayload = {
+    const payload: EventKeylessEnvDriftPayload = {
       publicKeyMatch,
       secretKeyMatch,
       envVarsMissing,
       keylessFileHasKeys,
-      keylessPublishableKey: keylessFile.publishableKey ?? '',
-      envPublishableKey: envPublishableKey ?? '',
+      keylessPublishableKeyMasked: maskKeyForTelemetry(keylessFile.publishableKey),
+      envPublishableKeyMasked: maskKeyForTelemetry(envPublishableKey),
     };
🧹 Nitpick comments (3)
packages/nextjs/src/server/keyless-telemetry.ts (2)

9-9: Use an event-specific flag file to avoid collisions and better reflect intent

Rename the flag file to be event-specific. This prevents future collisions if more telemetry one-shot flags are added and aligns naming with the event purpose.

-const TELEMETRY_FLAG_FILE = '.clerk/.tmp/telemetry.json';
+const TELEMETRY_FLAG_FILE = '.clerk/.tmp/keyless-telemetry-fired.json';

88-184: Add focused tests for drift scenarios and single-emission behavior

No tests added. Please cover:

  • Key presence matrix: only keyless keys present (no drift), neither present (no drift), both present and equal (no drift), both present and different (drift).
  • Publishable key env fallback: ensure CLERK_PUBLISHABLE_KEY is honored.
  • Partial env presence: only secret present and mismatched → drift; only publishable present and mismatched → drift.
  • Payload privacy: masked values present, no raw keys.
  • Single emission: concurrent calls only produce one record (simulate via mocking fs/promises writeFile with { flag: 'wx' } behavior).
  • Early return when no keyless file.

I can scaffold Jest tests with fs/promises and Clerk client mocks to validate these cases. Want me to draft them?

.changeset/bright-parks-search.md (1)

7-7: Update flag filename in the note if you adopt the event-specific flag file

If you change the code to use an event-specific file, mirror that here to keep docs consistent.

-This event only fires once as controlled with the .clerk/.tmp/telemetry.json file to prevent telemetry event noise
+This event only fires once via the .clerk/.tmp/keyless-telemetry-fired.json file to prevent telemetry event noise
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0c5b8fe and e5e7120.

📒 Files selected for processing (3)
  • .changeset/bright-parks-search.md (1 hunks)
  • packages/nextjs/src/app-router/server/ClerkProvider.tsx (1 hunks)
  • packages/nextjs/src/server/keyless-telemetry.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/nextjs/src/app-router/server/ClerkProvider.tsx
🧰 Additional context used
📓 Path-based instructions (8)
.changeset/**

📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)

Automated releases must use Changesets.

Files:

  • .changeset/bright-parks-search.md
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels

Files:

  • packages/nextjs/src/server/keyless-telemetry.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

Use Prettier for consistent code formatting

Files:

  • packages/nextjs/src/server/keyless-telemetry.ts
packages/**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

TypeScript is required for all packages

Files:

  • packages/nextjs/src/server/keyless-telemetry.ts
packages/**/*.{ts,tsx,d.ts}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

Packages should export TypeScript types alongside runtime code

Files:

  • packages/nextjs/src/server/keyless-telemetry.ts
**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)

Use proper TypeScript error types

**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoid any type - prefer unknown when type is uncertain, then narrow with type guards
Use interface for object shapes that might be extended
Use type for unions, primitives, and computed types
Prefer readonly properties for immutable data structures
Use private for internal implementation details
Use protected for inheritance hierarchies
Use public explicitly for clarity in public APIs
Prefer readonly for properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Use const assertions for literal types: as const
Use satisfies operator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports: import type { ... } from ...
No any types without justification
Proper error handling with typed errors
Consistent use of readonly for immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)

Files:

  • packages/nextjs/src/server/keyless-telemetry.ts
**/*.{js,ts,tsx,jsx}

📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)

Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.

Files:

  • packages/nextjs/src/server/keyless-telemetry.ts
**/*

⚙️ CodeRabbit Configuration File

If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.

Files:

  • packages/nextjs/src/server/keyless-telemetry.ts
🧬 Code Graph Analysis (1)
packages/nextjs/src/server/keyless-telemetry.ts (3)
packages/nextjs/src/server/keyless-node.ts (1)
  • safeParseClerkFile (52-66)
packages/nextjs/src/server/createClerkClient.ts (1)
  • createClerkClientWithOptions (34-35)
packages/types/src/telemetry.ts (1)
  • TelemetryEventRaw (40-44)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: Build Packages
  • GitHub Check: Formatting | Dedupe | Changeset
  • GitHub Check: semgrep/ci
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (2)
packages/nextjs/src/server/keyless-telemetry.ts (2)

135-147: Correctly avoid false positives by requiring presence on both sides before comparing

The match logic only treats values as matching when both env and keyless file values exist and are equal. This avoids undefined === undefined false positives.


168-179: Atomic single-emission guard is solid

Using an atomic create-if-not-exists and recording only on the winner avoids race conditions and duplicate telemetry. Directory creation before write is also handled.

Comment on lines +11 to +18
type EventKeylessEnvDriftPayload = {
publicKeyMatch: boolean;
secretKeyMatch: boolean;
envVarsMissing: boolean;
keylessFileHasKeys: boolean;
keylessPublishableKey: string;
envPublishableKey: string;
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Do not send raw keys; mask publishable keys and make fields optional

Raw publishable keys shouldn’t be sent in telemetry. Also, casting/forcing missing values into strings can hide absence. Make fields optional and masked, and prefer interface for an extendable payload shape per guidelines.

-type EventKeylessEnvDriftPayload = {
+interface EventKeylessEnvDriftPayload {
   publicKeyMatch: boolean;
   secretKeyMatch: boolean;
   envVarsMissing: boolean;
   keylessFileHasKeys: boolean;
-  keylessPublishableKey: string;
-  envPublishableKey: string;
-};
+  keylessPublishableKeyMasked?: string;
+  envPublishableKeyMasked?: string;
+}

Add this helper outside the shown range:

function maskKeyForTelemetry(key?: string): string | undefined {
  if (!key) return undefined;
  const head = key.slice(0, 8);
  const tail = key.slice(-4);
  return `${head}${tail}`;
}
🤖 Prompt for AI Agents
In packages/nextjs/src/server/keyless-telemetry.ts around lines 11 to 18, the
telemetry payload currently exposes raw publishable keys and forces values into
strings; change the type to an interface with optional fields (publicKeyMatch?:
boolean; secretKeyMatch?: boolean; envVarsMissing?: boolean;
keylessFileHasKeys?: boolean; keylessPublishableKey?: string;
envPublishableKey?: string) so absent values are not coerced, and remove any
code that sends raw keys; instead add the provided maskKeyForTelemetry helper
(outside this range) and set keylessPublishableKey and envPublishableKey to the
masked results (or undefined) before sending telemetry so raw keys are never
transmitted.

@heatlikeheatwave heatlikeheatwave merged commit 21c2956 into main Aug 12, 2025
39 checks passed
@heatlikeheatwave heatlikeheatwave deleted the heat/grow-664-investigate-10-drop-between-application-created-and-dev branch August 12, 2025 22:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants